Skip to content

Support longer relative paths in ResolveAssemblyReference HintPaths - #4414

Merged
rainersigwald merged 2 commits into
dotnet:masterfrom
mfilippov:fix-long-path-issue-in-hint-path
Jun 17, 2019
Merged

rainersigwald merged 2 commits into
dotnet:masterfrom
mfilippov:fix-long-path-issue-in-hint-path

Conversation

@mfilippov

Copy link
Copy Markdown
Contributor

This problem introduced in #3700. If hint path in reference plus current directory more than MAX_PATH then MSBuild couldn't resolve the reference. But file exists and the actual path less than MAX_PATH. The problem in function GetFileAttributesEx. This function just contacted hint path with the current directory, and it doesn't work when your current directory deep into file system but the library on the top of the hierarchy. I benchmarked this fix with BenchmarkDotNet and not found a statistically significant difference between the original code and my fix.

@mfilippov

Copy link
Copy Markdown
Contributor Author

Benchmark results:

BenchmarkDotNet=v0.11.5, OS=Windows 10.0.18362
Intel Core i7-6700 CPU 3.40GHz (Skylake), 1 CPU, 4 logical and 4 physical cores
.NET Core SDK=2.1.800-preview-009677
  [Host]     : .NET Core 2.1.11 (CoreCLR 4.6.27617.04, CoreFX 4.6.27617.02), 64bit RyuJIT
  DefaultJob : .NET Core 2.1.11 (CoreCLR 4.6.27617.04, CoreFX 4.6.27617.02), 64bit RyuJIT

Method Mean Error StdDev Ratio MannWhitney(10%) RatioSD Rank
HintPathResolverOld 25.97 us 0.4077 us 0.3814 us 1.00 Base 0.00 1
HintPathResolverNew 26.48 us 0.2821 us 0.2639 us 1.02 Same 0.02 2

As I understand it patch should reduce IO when you have different HintPath on the same file from different level project hierarchy.

@mfilippov mfilippov changed the title Fix problem when in hint path. Fix problem in hint path. Jun 3, 2019
Uses the already-existing centralized codepath to normalize paths

@rainersigwald rainersigwald left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great, thanks!

I pushed a change to use the standard-within-MSBuild method for this operation, which also allows removing the new using. I validated that the repro project from #4348 passes with my change.

@rainersigwald rainersigwald changed the title Fix problem in hint path. Support long, composite paths in ResolveAssemblyReference HintPaths Jun 11, 2019
@rainersigwald rainersigwald changed the title Support long, composite paths in ResolveAssemblyReference HintPaths Support longer relative paths in ResolveAssemblyReference HintPaths Jun 11, 2019
@mfilippov

Copy link
Copy Markdown
Contributor Author

@rainersigwald Thanks for the update. I didn't know about this API.

@rainersigwald
rainersigwald merged commit b63298b into dotnet:master Jun 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants